-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Migrate wasmtime-wasi to wasmtime::error
#12294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate wasmtime-wasi to wasmtime::error
#12294
Conversation
When doing plain old `cargo test -p wiggle` (or `wiggle-test`) the test would fail due to linking errors because Wasmtime's `std` cargo feature (and maybe others) wasn't being enabled. The crate's tests would pass when run as part of the whole workspace, however, because of cargo feature resolution and other crates that enabled the necessary features, which is why CI is green.
This is another interesting one because wasi-common is used internally to Wasmtime but also by other projects. Therefore, rather than using `wastime::error::*` and making `wasmtime` a hard dependency, we use just `wasmtime_environ::error`.
680d7d4 to
c89381f
Compare
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to rebase before merging to prune out the extra commits too? I don't think it'll matter in the end but it'll help clean up the merge commit message too
I rebased a few PRs down the line, which cut a bunch of earlier commits out, don't really want to do O(n) rebases for such a deep stack, especially when I would have to wait for each PR to merge before rebasing the next. As you say, this just ends up including some old commit messages in the merge commit message, so not that big a deal IMO. |
Builds on top of #12293